Skip to content

fix: CI#3206

Merged
chejinge merged 6 commits intoOpenAtomFoundation:3.5from
Z-G-H1:fix-ci-issue
Dec 16, 2025
Merged

fix: CI#3206
chejinge merged 6 commits intoOpenAtomFoundation:3.5from
Z-G-H1:fix-ci-issue

Conversation

@Z-G-H1
Copy link
Copy Markdown
Collaborator

@Z-G-H1 Z-G-H1 commented Dec 11, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 11, 2025 07:18
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 11, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the ☢️ Bug Something isn't working label Dec 11, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements disk space optimizations across CI workflows to address disk space constraints in GitHub Actions runners. The changes focus on aggressive cleanup strategies, updated build environments, and resource management improvements.

Key Changes:

  • Added disk space cleanup steps to free up storage on Ubuntu and Rocky Linux runners by removing unnecessary system files, build artifacts, and caches
  • Updated macOS runner from macos-13 to macos-14 and upgraded GCC from version 10 to version 13
  • Modified Git fetch depth and added binutils package installation for binary stripping operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

working-directory: ${{ github.workspace }}/build
run: |
rm -rf Testing CMakeFiles CMakeCache.txt
find . -name "*.o" -o -name "*.a" -delete
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern find . -name "*.o" -o -name "*.a" -delete may not work as expected. When using -o (OR) with -delete, the find command can exhibit unexpected behavior because -delete has lower precedence. Use -o within parentheses or use separate find commands for each file type to ensure both patterns are properly deleted.

Suggested change
find . -name "*.o" -o -name "*.a" -delete
find . \( -name "*.o" -o -name "*.a" \) -delete

Copilot uses AI. Check for mistakes.
cmake -B build -DCMAKE_C_COMPILER=/usr/local/opt/gcc@10/bin/gcc-10 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache
GCC_PREFIX=$(brew --prefix gcc@13)
export CC=$GCC_PREFIX/bin/gcc-13
cmake -B build -DCMAKE_C_COMPILER=$GCC_PREFIX/bin/gcc-13 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CMAKE configuration only specifies CMAKE_C_COMPILER but not CMAKE_CXX_COMPILER. For a C++ project (indicated by DCMAKE_CXX_FLAGS_DEBUG), both compilers should be explicitly set to ensure consistent toolchain usage. Add -DCMAKE_CXX_COMPILER=$GCC_PREFIX/bin/g++-13 to match the C compiler specification.

Suggested change
cmake -B build -DCMAKE_C_COMPILER=$GCC_PREFIX/bin/gcc-13 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache
cmake -B build -DCMAKE_C_COMPILER=$GCC_PREFIX/bin/gcc-13 -DCMAKE_CXX_COMPILER=$GCC_PREFIX/bin/g++-13 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/pika.yml Outdated
Comment on lines +231 to +232
go clean -cache -testcache
rm -rf "$HOME/.cache/go-build" "$HOME/go/pkg/mod"
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Go cache cleanup occurs before the "Run Go E2E Tests" step (which starts at line 243). This means the E2E tests will have to rebuild the Go cache from scratch, significantly slowing down the test execution. Move this cleanup to after all Go-related tests are complete, or remove it entirely if disk space permits.

Copilot uses AI. Check for mistakes.
@@ -256,6 +286,7 @@ jobs:
cp deps/lib/libz.1.dylib .
cp deps/lib/libz.1.dylib tests/integration/
rm -rf ./buildtrees
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sed command modifies test files to replace 'exec leaks' with 'exec echo "0 leaks"', effectively disabling memory leak detection in tests. While this may be intentional to avoid false positives or macOS-specific issues, consider documenting why leak detection is being disabled, as this could hide real memory leaks in the codebase.

Suggested change
rm -rf ./buildtrees
rm -rf ./buildtrees
# Disable memory leak detection on macOS tests by replacing 'exec leaks' with a no-op.
# This is done to avoid persistent false positives or compatibility issues with the 'leaks' tool on macOS CI.
# See https://github.com/pikadb/pika/issues/XXXX for more details. (Replace with actual issue if available.)

Copilot uses AI. Check for mistakes.
find ./build -name "*.a" -type f -delete

echo "Stripping debug symbols from binaries..."
find ./build -type f -executable -not -name "*.sh" -exec strip --strip-debug {} \; || true
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The strip command is used here, but it may fail if binutils is not properly installed or if the package name differs on Rocky Linux 9. The || true suffix masks potential failures. Consider verifying that the strip command is available after installing binutils, or use a conditional check before running strip to ensure proper error handling.

Suggested change
find ./build -type f -executable -not -name "*.sh" -exec strip --strip-debug {} \; || true
if command -v strip >/dev/null 2>&1; then
find ./build -type f -executable -not -name "*.sh" -exec strip --strip-debug {} \;
else
echo "Warning: 'strip' command not found. Skipping stripping debug symbols."
fi

Copilot uses AI. Check for mistakes.
Comment on lines +213 to +215
rm -rf .git
rm -rf include
rm -rf docs
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup step removes the .git directory and essential project directories (include, docs) which could break subsequent steps that need to access the git history or project files. These deletions happen before the Test and Unit Test steps, which may depend on these files. Consider moving these aggressive cleanup operations to after all tests and operations are complete, or verify that they won't break subsequent steps.

Copilot uses AI. Check for mistakes.
@chejinge chejinge merged commit 8afe87f into OpenAtomFoundation:3.5 Dec 16, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants